[feature][MSD-506] Meteor store annoted data - #3451

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/odemis/acq/feature.py`:
- Around line 573-584: The code currently sets channel_key = None when MD_OUT_WL
is missing, causing all such streams to collide and only the first to be kept;
change to use a per-stream fallback like the stream's position so missing
MD_OUT_WL yields a unique key. Iterate with an index over the concatenated list
(fm_zstacks + fm_images) and set channel_key =
s.raw[0].metadata.get(model.MD_OUT_WL, f"fallback:{i}") (or similar unique
identifier from the stream) before checking seen_channels; update references to
selected_fm and seen_channels accordingly.
- Around line 539-638: The race is that multiple threads can pass the initial if
not feature.collect check and all run until feature.collect is set False at the
end; make the one-shot flip atomic by performing an immediate compare-and-set at
the start of the routine (e.g., in collect_feature_data): replace the plain
boolean check of feature.collect with an atomic operation that sets
feature.collect to False only if it was True (or acquire a per-feature lock
keyed by feature id, check feature.collect and set it False while holding the
lock), and if the CAS/lock indicates collect was already False return early;
reference feature.collect and the top-level routine (collect_feature_data in
this file) when adding the atomic CAS or lock.

In `@src/odemis/acq/test/feature_test.py`:
- Around line 349-365: The test test_payload_has_no_feature_name only checks
that f.name.value ("my_secret_feature_name") is not present among payload keys;
update the assertions to also ensure the feature name does not appear in payload
values by checking captured.values() and the stringified values (for
nested/serialized values) after collect_feature_data runs — e.g., add assertions
using self.assertNotIn("my_secret_feature_name", captured.values()) and
self.assertNotIn("my_secret_feature_name", str(list(captured.values()))) to the
test (keep existing fake_record/captured usage and collect_feature_data
invocation).

In `@src/odemis/gui/cont/features.py`:
- Line 31: The module imports Dict and List from typing but uses Optional in the
annotation for self._status_collect_feature (type Optional[CryoFeature]) which
is undefined; update the import statement that currently lists Dict and List to
also import Optional so Optional is available for the annotation (refer to the
import line and the attribute self._status_collect_feature / CryoFeature usage
to locate the change).

In `@src/odemis/gui/cont/tabs/cryo_chamber_tab.py`:
- Around line 388-389: The code sets
self.tab_data_model.main.features_collectable using random.random() and
FEATURE_COLLECT_PROBABILITY but those names are not defined; import the random
module at the top of this module and add a module-level constant
FEATURE_COLLECT_PROBABILITY (e.g., a float like 0.1) before it’s used so
_change_project_conf() and any code referencing features_collectable (and
class/attribute names like tab_data_model.main.features_collectable) do not
raise NameError.

In `@src/odemis/gui/main.py`:
- Around line 436-441: The consent dialog updates consent via
self._data_collector.set_consent(...) but the Help > Share data menu checkbox
isn't refreshed; add a helper method refresh_data_sharing_state(self) in the
menu controller (e.g. class in src/odemis/gui/cont/menu.py) that does if
self._consent_menu_item is not None:
self._consent_menu_item.Check(self._data_collector.get_consent() is True), then
call that helper after the dialog result handling in main.py (after the branches
that call set_consent(True/False) or postpone_consent()) so the menu checkbox
reflects the new persisted consent state immediately.

In `@src/odemis/util/datacollector.py`:
- Around line 588-604: The loop currently does "if
self._process_pending_zips(...): continue" which starves new in-memory records;
change _run so that when _process_pending_zips(...) returns True you still
attempt to drain or process queued items instead of skipping the queue.get()
step — e.g., replace the continue with a non-blocking attempt to fetch work (use
self._queue.get_nowait() in a try/except queue.Empty or
self._queue.get(timeout=0.0)) and call self._process_work_item(item) if you get
one; keep the existing exception handling (_schedule_retry, logging.exception)
and avoid a tight busy-loop by falling back to the original blocking
self._queue.get(timeout=1.0) when no items are available.
- Around line 328-373: The event_name and payload keys are used directly to
build filesystem names (safe_event, zip_name, arc_name, extra_files paths) which
allows path traversal or unsafe ZIP entries; add a sanitization step that
normalizes item.event_name and each payload key into safe filenames (strip or
replace path separators like "/" and "\" and sequences like "..", collapse to a
whitelist of allowed chars such as alphanumerics, hyphen, underscore, enforce a
max length like 64) before using them to construct zip_name, tmp_dir children,
or arc_name; apply this sanitizer to safe_event, every arc_name (e.g., when
creating extra_{key}.json or {key}.h5/.ome.tiff) and when writing to tmp_dir or
adding to extra_files so no user-supplied string can escape tmp_dir or create
dangerous ZIP entries (update references in the code around safe_event,
zip_name, arc_name, payload_meta assignments, and extra_files population).

In `@src/odemis/util/dc_fetch.py`:
- Around line 40-47: The ISO parse path must normalize a trailing 'Z' before
calling datetime.fromisoformat to maintain Python 3.10 compatibility: in the
code handling text (the branch that calls datetime.fromisoformat(text)), detect
and replace a trailing 'Z' (or '+00:00' equivalent if present) with '+00:00' or
otherwise remove it so fromisoformat won't raise ValueError, then proceed to set
tzinfo to timezone.utc when parsed.tzinfo is None and use
parsed.astimezone(timezone.utc) when it has tzinfo; update the logic around the
parsed = datetime.fromisoformat(text) call accordingly.

---

Nitpick comments:
In `@src/odemis/gui/cont/menu.py`:
- Around line 46-54: Annotate the constructor and the new consent menu handlers
with explicit type hints: update __init__ to declare parameter types (e.g.,
main_data: MainGUIData, main_frame: wx.Frame, data_collector: DataCollector) and
the return type -> None; then locate the consent-related handler functions
referenced around lines 171-185 and add full parameter and return type
annotations (e.g., event: wx.Event or appropriate event type -> None, any other
params typed to their domain types). Ensure you import or reference the types
(MainGUIData, DataCollector, wx.Frame/Event) at the top of the module so the
annotations are valid.

In `@src/odemis/gui/win/consent.py`:
- Around line 16-59: Add reStructuredText docstrings to the Consent dialog
methods: document the __init__ constructor and each event handler method
(_on_opt_in, _on_opt_out, _on_remind_later, _on_close). For each docstring
include a one-sentence description of the method's purpose and, where helpful,
describe important parameters (e.g., evt) and the effect (which modal result is
returned) using plain text (no type info, no inline code/backticks), following
the project's reST style conventions.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/odemis/acq/feature.py`:
- Around line 614-627: Inspect DataCollector.record() and the metadata produced
by _get_raw() to determine which identifying fields survive export. Before
assigning DataArrays into payload, sanitize their metadata by removing
stream/channel names, filenames, acquisition-path fields, and other identifying
keys, while preserving non-identifying metadata and avoiding mutation of the
original raw data. Apply this consistently to selected_fm, overview_fm, and
overview_sem.

In `@src/odemis/util/datacollector.py`:
- Around line 343-361: In the ndarray export branch, update the archive name
construction to concatenate exporter.EXTENSIONS[0] directly without adding
another dot, and replace the hardcoded tiff.export call with the selected
exporter’s export method. Keep the existing format selection and error handling
unchanged.

In `@src/odemis/util/dc_fetch.py`:
- Around line 163-177: Update _write_dc_fetch_config to create or overwrite the
credentials INI with owner-only permissions (0600), rather than relying on the
process umask and plain "w" open. Preserve directory creation and config.write
behavior, ensuring existing files are also chmodded to 0600.

In `@util/release-odemis`:
- Around line 35-40: Update the source tarball generation around the git archive
HEAD command so the locally verified datacollector.key is explicitly added at
install/linux/usr/share/odemis/datacollector.key before compression. Preserve
the expected archive path and ensure the injected key is present in the
.orig.tar.gz uploaded to Launchpad.

---

Nitpick comments:
In `@src/odemis/util/dc_fetch.py`:
- Around line 179-264: Remove the duplicated default endpoint computation shared
by _load_or_init_dc_fetch_config and build_s3_client_from_config. Extract it
into a single module-level constant or small helper, then reuse that shared
value in both functions while preserving the existing None-to-empty-string
behavior.
- Around line 58-104: Extract the shared basename, ZIP-suffix, stem-splitting,
and part-count validation from parse_key_timestamp_utc and parse_key_event_name
into a _parse_key_stem helper returning the three parsed components or None.
Update both parsers to reuse this helper while preserving their existing
timestamp validation and event-name handling.

In `@src/odemis/util/test/dc_fetch_test.py`:
- Around line 77-172: Add tests for the skipped_existing and failed branches in
fetch_samples. Create a pre-existing destination file and assert it is skipped
and counted without downloading; separately make client.download_file raise an
exception, then assert failed is incremented and the temporary .part file is
removed. Reuse the existing mocked S3 client and temporary-directory setup in
the fetch_samples tests.

        decoded_features = [feature_decoder(f) for f in proj_data["features"]]
        for f in decoded_features:
            f.collect = False
        self.tab_data_model.main.features.value = [df for df in decoded_features if df is not None]

        # Decide once per project whether features created during this session are
        # eligible for data collection.  Stored as a dynamic attribute — not part
        # of the formal model — and read by add_new_feature via getattr.
        probability = DataCollector().probability
        self.tab_data_model.main.features_collectable = (

    collect = feature_raw.get('collect', False)
    feature = CryoFeature(name=feature_raw['name'],
                          stage_position=stage_position,
                          fm_focus_position=fm_focus_position,
                          collect=collect

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/odemis/acq/feature.py`:
- Around line 471-475: Update the load_feature_streams_from_disk call in the
feature stream-loading block to pass only the supported feature argument, while
preserving the existing exception handling and collection flow.

In `@src/odemis/gui/cont/tabs/cryo_chamber_tab.py`:
- Around line 396-399: Use a dedicated 20% sampling constant or initialized
query method in cryo_chamber_tab.py lines 396-399 for the per-project decision.
In datacollector.py lines 659-661, stop exposing the fixed default as a
ready-to-use computed rate; in datacollector.py lines 792-795, derive the rate
within the query method when consent-dependent sampling is required.
- Around line 528-531: Update the decoded-features handling around
feature_decoder so None results are filtered out before the loop mutates each
feature. Ensure f.collect is assigned only for valid decoded features, then
assign the filtered collection to main.features.value while preserving the
existing behavior for invalid entries.

---

Duplicate comments:
In `@src/odemis/acq/feature.py`:
- Around line 539-552: Update the payload-building loops in _export_data to
clone each non-null DataArray and remove identifying metadata, including
model.MD_DESCRIPTION, before assigning it to payload. Preserve the original
feature DataArrays unchanged and apply the sanitization consistently to
selected_fm, overview_fm, and overview_sem; add an artifact-level regression
test covering metadata redaction during export.

---

Nitpick comments:
In `@src/odemis/acq/feature.py`:
- Around line 174-189: Complete callable annotations and documentation across
the three affected sites: in src/odemis/acq/feature.py lines 174-189, annotate
CryoFeature.__init__ correlation_data with its appropriate type, add a None
return annotation, and remove type text from the changed docstring; in
src/odemis/acq/test/feature_test.py lines 97-292, add complete parameter and
return annotations plus concise docstrings to all added tests, helpers, and
callbacks; in src/odemis/gui/cont/tabs/cryo_chamber_tab.py lines 385-404, update
_change_project_conf to accept new_dir: str and return None.